Size Change: +4.32 kB (0%)

Total Size: 1.12 MB

_{compressed-size-action}

@youknowriad / @nosolosw Haii 👋

I'm trying to use the new style hooks with these padding controls. It was working a couple of days ago. I just checked and it looked like the inline style CSS variables aren't rendering anymore 🤔 .

I would love some insight/thoughts!

No rush at all! Thank you! <3

Very excited to be tapping into this style system layer.

How does the "Adds dimension controls to Group Block" PR: #16730 padding/margin controls
compare to the PR right here in adding padding styles to the cover block?

I am thinking there should be one margin/padding component/method to be used for all blocks.

Hi! I'm thinking about the conjunction of this PR with #21091. In particular: why present all four padding controls instead of just two contextual ones?

Let me explain: what if there were two controls that represent "distance to horizontal edge" and "distance to vertical edge"? Then, if the user selects grid position [0, 0], these settings correspond to padding-left and padding-top. Conversely, if the user selects grid position [2, 2], the settings correspond to padding-right and padding-bottom.

When grid position [1, 1] (i.e., centre-middle) is selected, then the user might either see no controls at all, or all four padding controls if they really want that option.

@ItsJonQ The issue is probably because we moved from CSS variables into inline styles. You should add your inline style mapping here https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/style.js#L45-L49

I think it's also worth noting, for accessibility reasons, the controls ought to be presented in a DOM/tab order that matches their visual order and makes sense. Going by that, the "All" input should come before or after the individual side inputs, rather than in the middle of them.

Personally, I think we should try to promote presets over one-off values, so setting the padding should be similar to setting a font size, where presets are provided alongside a "custom" option.

I think instead of 4 individual side controls + an all sides control, there should be 4 individual side controls + a toggle to lock horizontal values together + a toggle to lock vertical values together.

I also don't think it is necessary to present the options in a fancy cross layout. I think labels like "left" or "horizontal" can convey that just fine, and simplifying the presentation of the controls gives room for a UI more like the font size control to be implemented.

The issue is probably because we moved from CSS variables into inline styles. You should add your inline style mapping here https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/hooks/style.js#L45-L49

@youknowriad Ah, thank you so much! I was able to get the inline styles to connect with the controls. However, I noticed an issue...

Since the padding styles are being applied to the block-wrapper (really neat!), it renders to the block container div within the Edit UI, rather than the Cover itself:

The save output works correctly on the front-end 👍

The only solution that I can think of would involve rendering the values as CSS variables rather than direct style properties. This will enable me to "subscribe" and render those values within the inner Cover. This will work for both Edit and Save.

That being said, I'm open to any suggestions! <3

I think it's also worth noting, for accessibility reasons, the controls ought to be presented in a DOM/tab order that matches their visual order and makes sense.

@ZebulanStanphill Thanks for the suggestion! I just made that push.

Originally, I wanted to optimize the tab order to fit the mental model of how directional styles are rendered on the web (top, right, bottom, left). However, I recognize that it's a model that only people who have experience with it would have.

The tab order being tied to visual representation has a much stronger connection 👍

@ItsJonQ Actually, the best solution here is to refactor the Cover block to use the new lightBlockWrapper flag and the Block.* components in order to match the frontend markup in the editor. The alignments (float, wide...) can be hard to do right though. I also believe it's better to try to refactor the Cover block to lightBlockWrapper in its own PR to avoid mixing with the new additions of this PR. cc @ellatrix not sure if you've refactored blocks like the Cover block yet.

Actually, the best solution here is to refactor the Cover block to use the new lightBlockWrapper flag ... I also believe it's better to try to refactor the Cover block to lightBlockWrapper in its own PR...

@youknowriad Sounds good! For the time being, should we go with the CSS variable approach in this PR? Or should we pause on this one to wait for the refactor?

Thanks!

@ItsJonQ I'd rather try the lighterBlockWrapper first on a separate PR, maybe it's not that hard :)

@nosolosw Thanks for the feedback!! I'll take a looksy 👀

It looks like resetting only works per session, so if a user:

@nosolosw I guess this is a design decision!

Should "Reset" act like a "Clear"? (Where the values reset back to null) or...
Should it reset to the initial value during the session?

I'm okay with either! At the moment, like you pointed out, it's set to reset based on initial session value.

Thanks! <3

I think Reset should work to unset any user changes made. Mostly because if that's not the behavior, is there a way for a user to do that?

I think Reset should work to unset any user changes made

@jasmussen Gotcha! Thanks for the confirmation :). I'll work on making that happen 💪

is there a way for a user to do that?

At the moment, no. Which I suppose is why we need it to completely reset (or clear) changes :)

Good catch, Andres, and thanks for keeping on this Q, it's such a pleasure to watch you work.

@ItsJonQ another thing I've just noticed is that the padding attribute is stored directly as a key under style. However, the other style attributes are grouped such as:

• style.typography: fontSize, lineHeight
• style.color: link, text, background

If we're preparing the reset change for tomorrow's release, I guess it'd be good to consolidate the padding under the style.spacing namespace? That way, when we add the margin property we already have the namespace and don't need to introduce a block deprecation.

@nosolosw Thank you for the suggestion! I think the style.spacing namespace is a great idea

Feedback:
What I miss is having a way to see the padding as long as the cursor is focused in the spacing panel. Making a change I briefly for a half second see a very light blue color showing the padding. Having this light blue color show longer and atleast as long as the cursor is focused in inside either the panel or just inside the PX box would help to gain a visual view of padding size.

Another example.
When changing the numbers of pixels what about seeing the light blue padding color change size. A few seconds after having changed the size the light blue could disappear.

Showing the light blue padding overlay when focused/hovering on the associated input sounds like a neat idea. 👍

@nosolosw I have updates! I'm able to adjust the logic to handle reset (null values). Also! For the ability for the user to set zero (0) values, which is different than no user change.

Thanks for the suggestions @ZebulanStanphill + @paaljoachim !
I agree on the highlight / visualizers! I'll see what I can do 💪

Oh boy oh boy! 🎉

PR here :)
#23041

I'm using a lot of gutenberg controls on non gutenberg admin pages... This one, I'd like to use in a few places... what about allowing custom icons so we can do something a little like:

?

Interested if this is something I should pursue in adding to this, or just copy out the component and customise just for my project.

@rmorse Ooo. That's an interesting idea! 😄

I think we'd want to create some specific versions for some of these properties instead. For example, we have border radius in Button, but it looks nothing like this and we should be consistent internally and in the public component API

@mtias if you're referring to my mockups - this was just an idea, for my own personal use case - developing admin pages outside of the Gutenberg editor, using Gutenberg components (I guess this is the future workflow for WP admin in general - where theme / plugin authors will be reusing gutenberg components to build their own admin interfaces).

So what I want to be able to do is allow each dimension to be controlled individually, ( ie, the border radius properties ), rather than be "stuck" with Gutenberg best practise as in your example the radius control, because my use case is different - and this is a great component I could be repurposing.

I of course agree that it's necessary to keep consistency / good practice patterns within the Gutenberg editing experience, however I would love to keep the kind of flexibility that choosing custom icons would provide for this component, and as a general pattern / approach it would be great to bear in mind flexibiliy with components for developers as a whole, who will be building custom UIs now, and even more so further down the line using Gutenberg components

-I'm not sure you are actually suggesting to limit this flexibility... but wanted to share my perspective anyway :)

I'm nearly finished working on a plugin with a custom admin UI, and overall its been a fantastic experience dropping in Gutenberg components all over the place, saving me countless hours and making it 100x more accessible.